-
Notifications
You must be signed in to change notification settings - Fork 303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[circt-test] fix lit config for circt-bmc #7884
Conversation
fcf9bae
to
32e722d
Compare
32e722d
to
7a07ffb
Compare
One confusing point is that the Github CI can pass this test without path substitution and the additional PATH variable, which seems to be an existing CIRCT binary path in the PATH. |
Hmmm, this is interesting. The |
Ah sorry, I just now realized that this is for |
Yeah, it's a possible fix that I wasn't sure made sense before. Maybe we should just consider it a workaround to work with the Python script rather than using it to replace the substitution flow. |
integration_test/lit.cfg.py
Outdated
@@ -65,6 +65,7 @@ | |||
|
|||
# Tweak the PATH to include the tools dir. | |||
llvm_config.with_environment('PATH', config.llvm_tools_dir, append_path=True) | |||
config.environment['PATH'] = os.pathsep.join([config.circt_tools_dir, config.environment['PATH']]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TestingConfig should not have this member?
f21273c
to
d7a810c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice, thanks!
Would you like to merge this yourself, or do you want me to do it? |
I'd be really grateful if you could help me merge them for now. Thanks so much! |
399f68e
to
c723acf
Compare
Signed-off-by: unlsycn <unlsycn@unlsycn.com>
Oh..sorry, my wrong rebase screwed everything up! |
#7857 introduces a Python script to test
circt-bmc
, but there are two problems:lit.cfg.py
so that lit can correctly substitute the path in the test.circt-opt
but the executable is not in the PATH variable causing python cannot find it. This PR simply addscirct_tool_dir
to PATH to resolve it, but I think a better way is to get rid of the python script and allow lit to substitute the path.